Runner: add test for "out of memory" shutdown handler #1296
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jrfnl
force-pushed
the
php-8.5/runner-fix-out-of-memory-error-handling
branch
3 times, most recently
from
1ab76a9 to
622ea7b
Compare
|
Sorry about the commit noise, trying to get this working without being able to verify locally. |
jrfnl
force-pushed
the
php-8.5/runner-fix-out-of-memory-error-handling
branch
from
bd1edba to
ae20d34
Compare
PR squizlabs/PHP_CodeSniffer 3630 introduced a shutdown handler specifically to provide a more user-friendly error message when users run into "out of memory" errors. One of the challenges of doing this is making sure that enough memory is allocated for our own error message beforehand to make sure that the error message can still be created and displayed properly, even though PHP has already run out of memory. As of PHP 8.5, a new `fatal_error_backtraces` ini setting has been added and enabled by default, which adds a backtrace to fatal errors, which could have an impact on this memory allocation. > If the sub-vote to default to “1” passes, messages for fatal errors will now contain backtraces and may not match the format existing code is expecting. As this _may_ (or may not) impact the shutdown function in PHPCS, I'm adding some end-to-end tests to safeguard this functionality. Also reserving a little extra memory for the base message to be safer anyway. Ref: https://wiki.php.net/rfc/error_backtraces_v2#backward_incompatible_changes
jrfnl
force-pushed
the
php-8.5/runner-fix-out-of-memory-error-handling
branch
from
ae20d34 to
2ce2d1a
Compare
|
Also opened a related feature request for BashUnit: TypedDevs/bashunit#505 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
👉🏻 Note: this PR is pulled to the 3.x branch as runtime compatibility with new PHP versions is one of the few types of fixes still allowed to go into the 3.x branch. So if these tests would fail, the mitigation for that would need to go into the 3.x branch as a PHP runtime-compatibility fix.
PR squizlabs/PHP_CodeSniffer#3630 introduced a shutdown handler specifically to provide a more user-friendly error message when users run into "out of memory" errors. One of the challenges of doing this is making sure that enough memory is allocated for our own error message beforehand to make sure that the error message can still be created and displayed properly, even though PHP has already run out of memory.
As of PHP 8.5, a new
fatal_error_backtracesini setting has been added and enabled by default, which adds a backtrace to fatal errors, which could have an impact on this memory allocation.As this may (or may not) impact the shutdown function in PHPCS, I'm adding some end-to-end tests to safeguard this functionality.
Also reserving a little extra memory for the base message to be safer anyway.
Ref: https://wiki.php.net/rfc/error_backtraces_v2#backward_incompatible_changes
Suggested changelog entry
N/A (test only change - unless the tests would fail)